refactor: update GitHub workflows for release management and image build #97
Conversation
…ilding - Added new workflows for creating releases, preparing releases, and publishing libraries on merge. - Implemented logic to derive version from pull request titles and create Git tags/releases accordingly. - Enhanced image building workflows to include digest capturing and improved error handling. - Refactored existing workflows to streamline the process of bumping versions for internal libraries and services. - Introduced scripts for bumping chart versions and updating pyproject dependencies. - Removed obsolete scripts and workflows to clean up the repository.
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the release automation from a single monolithic workflow to a modular approach with label-based gating. It removes the legacy semantic-release workflow and introduces specialized workflows for each stage of the release process, along with new Python utilities for dependency and version management.
- Split release automation into 5 specialized workflows with label-based gating
- Added Python utilities for version bumping, dependency management, and chart updates
- Updated service names and versions to align with new release structure
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/update-helm-values.py | Removed legacy Helm values update script |
| tools/bump_pyproject_deps.py | Added utility for managing Python library versions and service dependency pins |
| tools/bump_chart_versions.py | Added utility for converting app versions to Helm chart versions |
| services/*/pyproject.toml | Updated service names and versions for consistency |
| .github/workflows/semantic-release.yml | Removed monolithic release workflow |
| .github/workflows/*.yml | Added modular workflows for prepare-release, publish-libs, build-images, create-release, and publish-chart |
| .github/workflows/lint-and-test.yml | Added label-based gating to skip CI during release automation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.github/workflows/publish-chart.yml
Outdated
| uses: azure/setup-helm@v4 | ||
|
|
||
| - name: Login to GHCR for Helm OCI | ||
| run: echo ${{ secrets.GHCR_PAT }} | helm registry login ghcr.io -u ${{ github.actor }} --password-stdin |
There was a problem hiding this comment.
The GHCR_PAT secret is being echoed directly in the pipeline output, which could expose the token in logs. Use the helm registry login command with the --password-stdin flag correctly by piping from a secure environment variable instead.
| run: echo ${{ secrets.GHCR_PAT }} | helm registry login ghcr.io -u ${{ github.actor }} --password-stdin | |
| run: printf "%s" "${{ secrets.GHCR_PAT }}" | helm registry login ghcr.io -u ${{ github.actor }} --password-stdin |
| for i, line in enumerate(lines): | ||
| stripped = line.strip() | ||
| if stripped.startswith('[tool.poetry]'): | ||
| in_tool_poetry = True | ||
| continue | ||
| if in_tool_poetry and stripped.startswith('[') and not stripped.startswith('[tool.poetry]'): | ||
| # left the section without finding version; stop scanning section | ||
| break | ||
| if in_tool_poetry and stripped.startswith('version'): | ||
| # Replace only the version value, keep indentation and spacing | ||
| lines[i] = re.sub(r'version\s*=\s*"[^"]*"', f'version = "{new_version}"', line) | ||
| return ''.join(lines) | ||
| # If no version line found, append it to the [tool.poetry] section |
There was a problem hiding this comment.
This fallback adds a duplicate [tool.poetry] section when no version line is found, which would create invalid TOML. The logic should append the version line to an existing [tool.poetry] section or handle the case where the section doesn't exist properly.
| for i, line in enumerate(lines): | |
| stripped = line.strip() | |
| if stripped.startswith('[tool.poetry]'): | |
| in_tool_poetry = True | |
| continue | |
| if in_tool_poetry and stripped.startswith('[') and not stripped.startswith('[tool.poetry]'): | |
| # left the section without finding version; stop scanning section | |
| break | |
| if in_tool_poetry and stripped.startswith('version'): | |
| # Replace only the version value, keep indentation and spacing | |
| lines[i] = re.sub(r'version\s*=\s*"[^"]*"', f'version = "{new_version}"', line) | |
| return ''.join(lines) | |
| # If no version line found, append it to the [tool.poetry] section | |
| tool_poetry_start = None | |
| tool_poetry_end = None | |
| version_found = False | |
| for i, line in enumerate(lines): | |
| stripped = line.strip() | |
| if stripped.startswith('[tool.poetry]'): | |
| in_tool_poetry = True | |
| tool_poetry_start = i | |
| continue | |
| if in_tool_poetry and stripped.startswith('[') and not stripped.startswith('[tool.poetry]'): | |
| # left the section without finding version; stop scanning section | |
| tool_poetry_end = i | |
| in_tool_poetry = False | |
| break | |
| if in_tool_poetry and stripped.startswith('version'): | |
| # Replace only the version value, keep indentation and spacing | |
| lines[i] = re.sub(r'version\s*=\s*"[^"]*"', f'version = "{new_version}"', line) | |
| version_found = True | |
| return ''.join(lines) | |
| # If [tool.poetry] section exists but no version line found, insert it | |
| if tool_poetry_start is not None: | |
| insert_at = tool_poetry_start + 1 | |
| # If we found the end of the section, insert before it | |
| if tool_poetry_end is not None: | |
| insert_at = tool_poetry_end | |
| # Determine indentation (match other lines in section, or default to 4 spaces) | |
| indent = "" | |
| for j in range(tool_poetry_start + 1, (tool_poetry_end or len(lines))): | |
| content = lines[j].lstrip('\r\n') | |
| if content and not content.strip().startswith('['): | |
| indent = lines[j][:len(lines[j]) - len(lines[j].lstrip())] | |
| break | |
| lines.insert(insert_at, f"{indent}version = \"{new_version}\"\n") | |
| return ''.join(lines) | |
| # If no [tool.poetry] section found, append it at the end |
…d dummy test module with basic functionality
… and clean semver enforcement
…refactoring for better publish control
a-klos
force-pushed
the
ci/refactor-ghas
branch
from
856845c to
d6dcc27
Compare
…ity and consistency
…ject.toml feat: enhance publish_libs.sh to automatically update rag-core-lib path dependency
…ntry and ensure correct versioning
…ase and promote-clean-semver workflows
…e-clean-semver workflow
…e-clean-semver workflow
…or dependency management
…low_dispatch for main branch
…ump script for PR label checks
…and API request handling
…eplace add_testpypi_source script
…rride and improve change detection logic
…api-lib, extractor-api-lib, and rag-core-api
| pyproject.write_text(new_text) | ||
| print(f"Rewrote rag-core-lib path dependency in {pyproject} to =={version}") | ||
| PY | ||
| fi |
There was a problem hiding this comment.
The bash script uses 'set -euo pipefail' but then calls functions that may fail without proper error handling. Specifically, 'poetry publish' on line 33 may fail if the package already exists on the index, causing the script to exit unexpectedly. Consider adding explicit error handling or checking if the version already exists before attempting to publish.
| fi | |
| fi | |
| # Skip publishing if this version already exists on the target index. | |
| local base_url="https://pypi.org" | |
| if [ "${repo_flag:-}" = "-r testpypi" ]; then | |
| base_url="https://test.pypi.org" | |
| fi | |
| if curl -fsSL "$base_url/pypi/$lib/json" -o /tmp/"$lib".json; then | |
| if jq -e --arg v "$version" '.releases[$v] | length > 0' /tmp/"$lib".json >/dev/null; then | |
| echo "$lib==$version already exists on ${repo_flag:-pypi default}; skipping publish" | |
| return 0 | |
| fi | |
| fi |
| def ensure(mapping: Dict[str, Any], key: str) -> Dict[str, Any]: | ||
| """Ensure key exists and is a dict.""" | ||
| if key not in mapping or mapping[key] is None: | ||
| mapping[key] = {} | ||
| if not isinstance(mapping[key], dict): | ||
| raise TypeError(f"Expected dict at {key}, got {type(mapping[key])}") | ||
| return mapping[key] | ||
|
|
||
|
|
||
| def update_values(values_path: Path, image_registry: str, image_tag: str) -> None: | ||
| if values_path.exists(): | ||
| data = yaml.safe_load(values_path.read_text(encoding="utf-8")) or {} | ||
| else: | ||
| data = {} | ||
|
|
||
| components = { | ||
| "backend": "rag-backend", | ||
| "adminBackend": "admin-backend", | ||
| "extractor": "document-extractor", | ||
| "frontend": "frontend", | ||
| "adminFrontend": "admin-frontend", | ||
| } | ||
|
|
||
| for key, image_name in components.items(): | ||
| comp = ensure(data, key) | ||
| image_block = ensure(comp, "image") | ||
| image_block["repository"] = f"{image_registry}/{image_name}" | ||
| image_block["tag"] = image_tag | ||
|
|
||
| backend = ensure(data, "backend") | ||
| mcp = ensure(backend, "mcp") | ||
| mcp_image = ensure(mcp, "image") | ||
| mcp_image["repository"] = f"{image_registry}/mcp-server" | ||
| mcp_image["tag"] = image_tag | ||
|
|
||
| values_path.write_text(yaml.safe_dump(data, sort_keys=False), encoding="utf-8") | ||
|
|
||
|
|
||
| def main() -> None: | ||
| parser = argparse.ArgumentParser(description="Update image overrides in a values file.") | ||
| parser.add_argument("--values-file", required=True, help="Path to values-qa.yaml in deployment repo") | ||
| parser.add_argument("--image-registry", required=True, help="Image registry base (e.g. registry.onstackit.cloud/qa-rag-template)") | ||
| parser.add_argument("--image-tag", required=True, help="Image tag/version to set") | ||
| args = parser.parse_args() | ||
|
|
||
| update_values(Path(args.values_file), args.image_registry, args.image_tag) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
There was a problem hiding this comment.
Missing type hints for function return values. The functions 'ensure', 'update_values', and 'main' should specify return types for better type safety and code clarity. Following the repository's Python conventions, all functions should have complete type annotations.
| def get_dependencies(doc: tomlkit.TOMLDocument): | ||
| ref = doc | ||
| for key in ["tool", "poetry", "dependencies"]: | ||
| if key not in ref: | ||
| return None | ||
| ref = ref[key] | ||
| return ref | ||
|
|
||
|
|
||
| def set_rag_core_lib_source(pyproject: Path) -> bool: | ||
| if not pyproject.exists(): | ||
| return False | ||
|
|
||
| doc = tomlkit.parse(pyproject.read_text()) | ||
| deps = get_dependencies(doc) | ||
| if deps is None or RAG_CORE_LIB not in deps: | ||
| return False | ||
|
|
||
| val = deps[RAG_CORE_LIB] | ||
| if isinstance(val, str): | ||
| it = tomlkit.inline_table() | ||
| it.add("version", val) | ||
| it.add("source", "testpypi") | ||
| deps[RAG_CORE_LIB] = it | ||
| elif hasattr(val, "get"): | ||
| if "path" in val: | ||
| return False | ||
| val["source"] = "testpypi" | ||
| else: | ||
| return False | ||
|
|
||
| doc["tool"]["poetry"]["dependencies"] = deps | ||
| pyproject.write_text(tomlkit.dumps(doc)) | ||
| return True | ||
|
|
||
|
|
||
| def main() -> int: | ||
| for pyproject in LIB_PYPROJECTS: | ||
| set_rag_core_lib_source(pyproject) | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| raise SystemExit(main()) |
There was a problem hiding this comment.
Missing docstrings for functions. According to the repository's coding guidelines, all functions should have NumPy-style docstrings documenting parameters, return values, and purpose. Functions like 'get_dependencies', 'set_rag_core_lib_source', and 'main' lack proper documentation.
| echo "$name==$version visible on $label" | ||
| return 0 | ||
| fi | ||
| sleep 5 |
There was a problem hiding this comment.
The hardcoded 5-second sleep on line 59 in the wait loop is a magic number. This should be extracted as a constant (e.g., 'POLL_INTERVAL_SECONDS = 5') at the top of the script for better maintainability and clarity.
| req.add_header("Authorization", f"Bearer {token}") | ||
| req.add_header("Accept", accept or "application/vnd.github+json") | ||
| req.add_header("X-GitHub-Api-Version", API_VERSION) | ||
| with urllib.request.urlopen(req) as resp: |
There was a problem hiding this comment.
The workflow uses 'urllib.request.urlopen' without timeout configuration, which can cause the workflow to hang indefinitely if the GitHub API is slow or unresponsive. Add a timeout parameter to prevent hanging, e.g., 'urllib.request.urlopen(req, timeout=30)'.
| def bump_chart(chart_path: pathlib.Path, app_version: str, mode: str): | ||
| yaml = YAML() | ||
| yaml.preserve_quotes = True | ||
| yaml.width = 4096 | ||
|
|
||
| data = yaml.load(chart_path.read_text()) | ||
| if data is None: | ||
| return | ||
|
|
||
| if mode in ("app-and-chart", "app-only"): | ||
| if not app_version: | ||
| raise ValueError("app_version is required for mode app-and-chart or app-only") | ||
| old_app = data.get("appVersion") | ||
| data["appVersion"] = _preserve_style(old_app, str(app_version)) | ||
|
|
||
| if mode in ("app-and-chart", "chart-only"): | ||
| if mode == "chart-only" and not app_version: | ||
| raise ValueError("chart-only mode requires chart_version provided via app_version argument") | ||
| chart_version = _to_chart_version(str(app_version)) | ||
| old_version = data.get("version") | ||
| data["version"] = _preserve_style(old_version, chart_version) | ||
|
|
||
| with chart_path.open("w") as handle: | ||
| yaml.dump(data, handle) |
There was a problem hiding this comment.
Missing error handling for YAML parsing operations. If the Chart.yaml file is malformed or missing expected fields, the script will fail with unclear error messages. Add try-except blocks around YAML operations with descriptive error messages to improve debugging experience.
| AUTH_URL="${DEPLOY_REPO_URL/https:\/\//https://${DEPLOY_REPO_TOKEN}@}" | ||
| git clone --depth 1 --branch "${DEPLOY_REPO_BRANCH:-main}" "$AUTH_URL" /tmp/deploy-repo | ||
|
|
||
| - name: Update values file in deployment repo | ||
| env: | ||
| IMAGE_REGISTRY: ${{ env.IMAGE_REGISTRY }} | ||
| IMAGE_TAG: ${{ env.IMAGE_TAG }} | ||
| DEPLOY_VALUES_FILE: ${{ secrets.DEPLOY_VALUES_FILE || 'values-qa.yaml' }} | ||
| run: | | ||
| set -euo pipefail | ||
| cd /tmp/deploy-repo | ||
| python -m pip install --quiet pyyaml | ||
| python "$GITHUB_WORKSPACE/tools/update_deploy_values.py" \ | ||
| --values-file "$DEPLOY_VALUES_FILE" \ | ||
| --image-registry "$IMAGE_REGISTRY" \ | ||
| --image-tag "$IMAGE_TAG" | ||
|
|
||
| - name: Commit and push deployment repo changes | ||
| env: | ||
| DEPLOY_REPO_BRANCH: ${{ secrets.DEPLOY_REPO_BRANCH || 'main' }} | ||
| DEPLOY_REPO_TOKEN: ${{ secrets.DEPLOY_REPO_TOKEN }} | ||
| DEPLOY_REPO_URL: ${{ secrets.DEPLOY_REPO_URL }} | ||
| DEPLOY_GIT_USER_NAME: ${{ secrets.DEPLOY_GIT_USER_NAME || 'github-actions' }} | ||
| DEPLOY_GIT_USER_EMAIL: ${{ secrets.DEPLOY_GIT_USER_EMAIL || '[email protected]' }} | ||
| run: | | ||
| set -euo pipefail | ||
| cd /tmp/deploy-repo | ||
| git config user.name "${DEPLOY_GIT_USER_NAME}" | ||
| git config user.email "${DEPLOY_GIT_USER_EMAIL}" | ||
| git add . | ||
| if git diff --cached --quiet; then | ||
| echo "No changes to commit" | ||
| exit 0 | ||
| fi | ||
| git commit -m "chore: update QA images to ${IMAGE_TAG}" | ||
| AUTH_URL="${DEPLOY_REPO_URL/https:\/\//https://${DEPLOY_REPO_TOKEN}@}" | ||
| git push "$AUTH_URL" "HEAD:${DEPLOY_REPO_BRANCH:-main}" |
There was a problem hiding this comment.
Security concern: The workflow exposes sensitive tokens in environment variables that could potentially be logged. The DEPLOY_REPO_TOKEN on lines 239, 259, and 274 is used to construct authentication URLs. Consider using GitHub Actions' built-in secret masking more carefully or using git credential helpers instead of embedding tokens in URLs.
| echo "Could not derive version from PR title: $TITLE" >&2 | ||
| exit 1 | ||
| fi | ||
| CLEAN_VERSION="${VERSION%%.post*}" |
There was a problem hiding this comment.
The regex pattern for version extraction on line 59 allows optional '.post' suffix but the clean_version derivation on line 64 may not handle all edge cases correctly. If the version format changes or contains unexpected characters, the sed command could produce incorrect results. Consider using a more robust parsing approach or adding validation of the extracted version format.
| CLEAN_VERSION="${VERSION%%.post*}" | |
| if ! echo "$VERSION" | grep -Eq '^[0-9]+\.[0-9]+\.[0-9]+(\.post[0-9]+)?$'; then | |
| echo "Derived version has invalid format: $VERSION" >&2 | |
| exit 1 | |
| fi | |
| CLEAN_VERSION=$(echo "$VERSION" | sed -E 's/^([0-9]+\.[0-9]+\.[0-9]+)(\.post[0-9]+)?$/\1/') |
This pull request restructures and improves the release automation for the repository by splitting the previous monolithic workflow into several specialized GitHub Actions workflows. These new workflows automate versioning, library publishing, Docker image building, Helm chart packaging, and dependency lockfile refreshing. Additionally, the workflows now use label-based gating to control releases and dependency updates, and they improve version extraction and artifact handling. The previous workflow file
.github/workflows/semantic-release.ymlhas been removed and replaced with more maintainable, modular workflows.Release and Versioning Automation
.github/workflows/prepare-release.ymlto automate semantic version calculation and bump internal library versions, opening a PR with the new versions and gating future steps with theprepare-releaselabel..github/workflows/create-release.ymlto create a Git tag and GitHub Release when a PR with therefresh-lockslabel is merged tomain, extracting the version from the PR title.Library Publishing and Dependency Management
.github/workflows/publish-libs-on-merge.ymlto publish Python libraries to TestPyPI after aprepare-releasePR is merged, update service dependency pins, refresh lockfiles, and open a PR with updated lockfiles using therefresh-lockslabel.Docker Image and Helm Chart Automation
.github/workflows/build-images.ymlto build and publish Docker images for all services when a release is published, capturing image digests as artifacts..github/workflows/publish-chart.ymlto package and publish the Helm chart after images are built, bumping chart versions and opening a PR for chart version updates using thechart-bumplabel.Workflow Gating and Cleanup
.github/workflows/lint-and-test.ymlto skip jobs if any of the release-related labels (prepare-release,refresh-locks,chart-bump) are present, preventing unnecessary CI runs during release automation..github/workflows/semantic-release.ymlworkflow, which previously handled all release steps in one file, in favor of the new modular approach.These changes collectively make the release process more robust, modular, and maintainable, while ensuring that versioning, publishing, and dependency updates are tightly controlled and automated.